-
Notifications
You must be signed in to change notification settings - Fork 0
[#22] 작품 회차 등록 #29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[#22] 작품 회차 등록 #29
Conversation
|
f-lab-saponin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드작성하시느라 고생하셨습니다!
전반적으로 파일수가 많은데, 가능하면 줄여보면 좋을 것 같습니다.
사용하지 않는 부분은 과감히 제거하구요.
추가로 *Service 에 있는 공개메소드마다 테스트를 만들어주시면 더 안전한 코드를 만들 수 있어보여요.
댓글한번 확인해주세요 감사합니다 👍👍
| @Transactional | ||
| public EpisodeResponse publishEpisode(EpisodeRequest req) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기서는 Episode + ManuscriptImages + EpisodeThumbnails 가 함께 되면 좋아보이는데, 트랜잭션 설정하는건 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드를 전부 살펴보고, @transactional 처리도 되어있어서 어떤 설정을 말씀하시는 건지 이해가 안 가는데 혹시 어떤 부분을 말씀하시는 걸까요? 😅
| @Transactional // 더티체킹 | ||
| public List<FileObjectResponse> checkAndGetStatus(Long fileObjectId) { | ||
| FileObject original = fileObjectRepository.findById(fileObjectId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여긴 조회만 하는 부분인것 같아요. 그래서 필요없을 것 같습니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
바뀐 10b980e 코드 기준 아래와 같이 엔티티로 업데이트 하는 부분이 있어서 더티체킹 용도로 사용했습니다!
.forEach(fo -> {
...
if (missingSet.contains(key)) fo.markFailed();
else fo.markReady();
fo.markSize(sizeBytes);
});
| List<FileObject> findByStorageKeyIn(Collection<String> storageKeys); | ||
| List<FileObject> findByStorageKeyStartingWith(String baseKey); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
쿼리 작성시, 적절한 인덱스를 고려해주세요. 인덱스 역시 히스토리로 어떤 파일에 기록하면 좋아요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
store_key 컬럼에 유니크 인덱스(uk_file_object_storage_key)가 적용되어 있었고, 2개 쿼리문을 EXPLAIN을 사용해 실행한 결과 해당 인덱스를 사용함을 확인했습니다. 차후 Flyway 적용시 기록해 놓겠습니다.
작품 등록, 회차 부분 전체적으로 코드 정리 진행하겠습니다. |
f-lab-saponin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다!! 승인드립니다 감사합니다!



작업 내용
테스트 내용
추가 필요